GH-36209: [Java] Upgrade Netty due to security vulnerability #36211

BryanCutler · 2023-06-21T18:47:03Z

Rationale for this change

Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845
This also requires a patch to arrow-memory

What changes are included in this PR?

Upgrading Netty, gRPC and Protobuf dependencies

Are these changes tested?

Existing tests

Are there any user-facing changes?

No

This PR contains a "Critical Fix".

netty-handler SniHandler 16MB allocation

The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap.

GHSA-6mjq-h674-j845

Closes: [Java] Upgrade Netty due to CVE #36209
GitHub Issue: [Java] Upgrade Netty due to CVE #36209

BryanCutler · 2023-06-21T18:48:33Z

@lidavidm this is a bit of a problem because Arrow memory needed to be patched to use the fixed version. Is there any discussion of doing a 12.0.2 release that could include this?

lidavidm · 2023-06-21T18:52:41Z

I think 12.0.2 is unlikely, given the 13.0.0 code freeze is a few weeks away, but if you raise it on the ML we can see.

I think this can affect arrow flight, given gRPC uses Netty.

BryanCutler · 2023-06-21T19:59:24Z

Thanks @lidavidm !

conbench-apache-arrow · 2023-06-23T08:46:17Z

Conbench analyzed the 6 benchmark runs on commit ea4f03ac.

There were 3 benchmark results indicating a performance regression:

Commit Run on ursa-thinkcentre-m75q at 2023-06-22 21:47:35Z
- params=1048576/1, source=cpp-micro, suite=arrow-acero-aggregate-benchmark
Commit Run on arm64-m6g-linux-compute at 2023-06-22 00:22:05Z
- params=131072/2, source=cpp-micro, suite=arrow-bit-util-benchmark
and 1 more (see the report linked below)

The full Conbench report has more details.

…pache#36211) Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845 This also requires a patch to arrow-memory Upgrading Netty, gRPC and Protobuf dependencies Existing tests No **This PR contains a "Critical Fix".** netty-handler SniHandler 16MB allocation The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap. GHSA-6mjq-h674-j845 * Closes: apache#36209 Authored-by: Bryan Cutler <[email protected]> Signed-off-by: David Li <[email protected]>

…pache#36211) (#27) * apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211) Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845 This also requires a patch to arrow-memory Upgrading Netty, gRPC and Protobuf dependencies Existing tests No **This PR contains a "Critical Fix".** netty-handler SniHandler 16MB allocation The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap. GHSA-6mjq-h674-j845 * Closes: apache#36209 Authored-by: Bryan Cutler <[email protected]> Signed-off-by: David Li <[email protected]> * Restore jackson version. --------- Signed-off-by: David Li <[email protected]> Co-authored-by: Bryan Cutler <[email protected]>

* apacheGH-36209: [Java] Upgrade Netty due to security vulnerability (apache#36211) Upgrading Netty dependency due to CVE GHSA-6mjq-h674-j845 This also requires a patch to arrow-memory Upgrading Netty, gRPC and Protobuf dependencies Existing tests No **This PR contains a "Critical Fix".** netty-handler SniHandler 16MB allocation The SniHandler can allocate up to 16MB of heap for each channel during the TLS handshake. When the handler or the channel does not have an idle timeout, it can be used to make a TCP server using the SniHandler to allocate 16MB of heap. GHSA-6mjq-h674-j845 * Closes: apache#36209 Authored-by: Bryan Cutler <[email protected]> Signed-off-by: David Li <[email protected]> * Restore jackson version. * Use local based ccache for Mac build instead of sscache. --------- Signed-off-by: David Li <[email protected]> Co-authored-by: Bryan Cutler <[email protected]>

idelpivnitskiy · 2023-07-06T17:17:09Z

What is ETA for 13.0.0 release?

lidavidm · 2023-07-06T18:26:05Z

The code freeze is proposed to be July 10th (https://lists.apache.org/thread/f9r0dsd65ohdtcvc7fnnlfs23n3z0n7f). It would then be one to several weeks to chase down release blockers, prepare binaries, and vote on the release, depending on if any last-minute issues are found.

Generally releases are conducted quarterly.

idelpivnitskiy · 2023-07-07T18:52:14Z

Thanks for response @lidavidm,
I understand your general release cadence, but would like to highlight that this incompatibility does not allow upgrading Netty version for anything else in the same classpath. It will be great for the project if Arrow can find a way to either backport it to older versions (maybe MethodHandlers with runtime Netty version check can help), or find a way to stop using internal Netty API completely (preferred approach).

In the meanwhile, are there any known downsides or issues for users switching from arrow-memory-netty to arrow-memory-unsafe? I couldn't find much information about side-effects. Assuming that they are interchangeable.

lidavidm · 2023-07-07T19:01:03Z

We would love to be more agile with releases. However, it takes quite a bit of maintainer effort. Help is welcome.

@danepitkin was exploring if reflection could help. If you have ideas on how exactly that could work, that may be useful. I'm not familiar with why the code is structured like this, other than performance (but I'm not aware of benchmarks for this).

arrow-memory-unsafe has not received as much usage, in my estimation, because netty was for a long time the only implementation and is still the 'default' in effect. Also, if your application also uses Netty, the unsafe implementation is not aware of memory allocated by Netty and vice versa (may have to tweak the JVM native memory limit).

lidavidm · 2023-07-07T20:03:08Z

I filed apache/arrow-java#184 to investigate this in the future.

(Though you gave me an idea; maybe we could make InnerAllocator here an interface and dispatch between a naive version and an internals-using version at runtime; presumably the JIT could devirtualize the call over time.)